-
Notifications
You must be signed in to change notification settings - Fork 0
feat: 게스트 로그인 구현 #12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: 게스트 로그인 구현 #12
Conversation
| @RequiredArgsConstructor | ||
| public class JwtAuthenticationFilter extends OncePerRequestFilter { | ||
| private final UserDetailsService userDetailsService; | ||
| private final UserDetailsServiceImpl userDetailsService; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UserDetailsService를 상속받는 GuestDetailsService Interface를 작성 후에
GuestDetailsService guestDetailsService;
UserDetailsService userDetailsService;
if (role.equals(String.valueOf(Role.GUEST))) {
Long guestId = tokenProvider.getUserId(accessToken);
userDetails = guestDetailsService.loadGuestByUserId(guestId);
}
else{
String username = tokenProvider.getUsername(accessToken);
userDetails = userDetailsService.loadUserByUsername(username);
}
요런 식으로 하는게 책임분리도 되고 좋지 않을까용? 아직 서비스 크기가 크진 않으니까 UserDetailsServiceImpl에 몰아도 상관 없을 것 같지만, Guest와 User가 성격 자체가 조금은 다르니 별도의 DetailsService에서 처리해주어도 좋다고 생각합니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guest와 User가 완전히 다른 엔터티가 아니고 같은 엔터티니까 UserDetailsService에서 전부 처리하는 것도 맞는 것 같지만, 그렇다고 loadUserByUsername() 함수 안에 Guest, User에 대한 로직을 합쳐서 username 조건에 따라 분기하는 방법을 쓰기에는 Guest는 unique한 email이 저장되지 않으니 좋은 방법은 아닌 것 같습니다.
하나의 UserDetailsService.loadUserByUsername() 에서 전부 로직을 처리하려면
public TokenDto createGuestToken(User guest){
Long userId = guest.getId();
String username = guest.getNickname(); // "guest"
return getTokenDto(guest, userId, username);
}
여기서 username 대신에 "guest" 를 넣어서 loadUserByUsername에서 username.equals("guest") 로 분기를 하는 방법도 가능할 것 같습니당!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
모든 방법이 전부 근거가 있는 것 같아서 같이 논의해보면 좋을 것 같습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
맞습니다! 저도 분리하는게 더 나은 방법인 것 같긴합니다
민주님도 동의하신다면 바로 수정하도록 하겠습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
게스트 토큰을 만들 때, usetName에 게스트임을 구분할 수 있게 "guest"라는 고정된 값을 넣자는 게 좋은 의견인 것 같아! 로컬에서 한 번 구현을 해보았습니다. 가능할 것 이라고 생각하나, 직접 구현을 해보니 제가 생각하기에 해당 로직은 다음과 같은 단점을 지녔던 것 같습니다.
단순 저의 생각으로 구현을 해본 것임을 감안해주세요!
현재 방법 : userDetailsService 함수 하나로 전부 처리해보자!
1. token 생성 단계
public TokenDto createGuestToken(User guest){
Long userId = guest.getId();
String username = GUEST + userId // GUEST = "guest";
return getTokenDto(guest, userId, username);
}2. JwtAuthenticationFilter는 코드 변경 X
UserDetails userDetails = userDetailsService.loadUserByUsername(username);3. UserDetailsServiceImpl
@Service
@RequiredArgsConstructor
public class UserDetailsServiceImpl implements UserDetailsService {
private final UserRepository userRepository;
private final RedisUtil redisUtil;
@Override
@Cacheable(value = "users", key = "#username")
public UserDetails loadUserByUsername(String username) throws UsernameNotFoundException {
if(username.startsWith(GUEST)) { //만약 게스트 로그인인 경우, 현재 해당 값은 GUEST+uuid(guestId)
return loadGuestByGuestId (guesetId); //userName에서 prefix GUEST 떼고 UUID 만 넣음
}
User user = userRepository.findByEmail(username)
.orElseThrow(() -> new UsernameNotFoundException("User not found : " + username));
return new SecurityUserDetails(user);
}
private UserDetails loadGuestByGuestId(String id){
User guest = Optional.ofNullable((User) redisUtil.getData("guest:" + guestId))
.orElseThrow(()-> new UsernameNotFoundException("Guest not found : " + guestId));
redisUtil.setDataExpire("guest:" + guestId, GUEST_TTL_SECONDS);
return new SecurityUserDetails(guest);
}
}loadUserByUsername은 제공되는 함수라 인자를 userName밖에 못 받아서 String 파싱을 통해 구현함.
다음과 같이 코드를 변경 했을 때 다음과 같은 장점을 지니는 것 같습니다.
- JwtAuthenticationFilter가 간단함.
- 회원 / 비회원 별 클래스를 따로 구현하지 않아 클래스 구조가 단순
- 게스트와 일반 유저 구분이 username으로 드러나 직관적
그러나 단점이 더 큽니다.
- Spring Security의 원칙 및 장점은 권한 기반 인가 시스템임.
-> 그러나 String 파싱을 통한 코드 구현을 하면 우리만의 새로운 인가 흐름이 생기는 것.
-> 본인 외 다른 개발자들이 보기에는 자연스럽지 못한 코드 흐름으로 가독성 측면에서 좋지 않음. - srp를 지키지 못함. 유저용 UserDetails를 반환하던 함수에서 더 추가되어 유저용/비유저용 객체 반환해야 함
-> 각 케이스 별 로직 처리도 달라, 사실상 하나의 함수가 너무 많은 일을 하는 중
-> 만약 여기서 권한이 더 늘어나야 한다면 하드코딩 + 가독성 최악 + srp 파괴 의 함수가 될 것으로 예상
결론 : JwtAuthentication에서 토큰 내 권한으로 분기처리를 하는 방법 추천.
형준 선배의 첫 번째 의견처럼
GuestDetailsService guestDetailsService;
UserDetailsService userDetailsService;
이 srp 원칙 준수 + 권한 기반 인가 시스템 로직 유지 + 가독성 최고
일 것 같습니다.
| private final AntPathMatcher antPathMatcher = new AntPathMatcher(); | ||
|
|
||
| public JwtAuthenticationFilter( | ||
| @Qualifier("userDetailsServiceImpl") UserDetailsService userDetailsService, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
깔끔한 빈 관리 좋네요!!
# Conflicts: # src/main/java/Gotcha/common/jwt/token/JwtHelper.java
| User guest = Optional.ofNullable((User) redisUtil.getData("guest:" + guestId)) | ||
| .orElseThrow(()-> new UsernameNotFoundException("Guest not found : " + guestId)); | ||
|
|
||
| redisUtil.setDataExpire("guest:" + guestId, GUEST_TTL_SECONDS); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기 "guest:" 라는 prefix는, authService에서도 동일하게 사용되고 있는데, 하드코딩으로 넣어주기 보단 상수로 정의해서 관리해주면 유지보수 측면에서 좋을 것 같습니다!
brothergiven
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고하셨습니다 !! 좋은 코드 잘 보고 갑니다👍
👤게스트 로그인 구현
📝 개요
게스트 로그인을 구현하였습니다.
⚙️ 구현 내용
이를 통해 게스트로 플레이 도중 회원가입을 통해 사용자로의 데이터 전환이 더욱 편해질 것으로 예상됩니다.
또한, 게스트와 사용자를 구분하는 과정에서도 (feat: 게스트 로그인 구현 #11)과 달리 아래처럼 사용자와 게스트 두가지를 통합적으로 관리할 수 있습니다.
📎 기타
기존에는 위와 같이 JwtAuthenticationFilter에서 UserDetailsService를 사용했지만 위와 같은 로직을 사용하기 위해서
아래와 같이 UserDetailsServiceImpl을 사용하게 되었습니다.
이게 괜찮은건지 의문입니다. 다른 분들의 의견이 궁금합니다
🧪 테스트 결과